spec/mailtrap/email_log_message_spec.rb (1)

27-29: Test intent says “all attributes” but excludes optional fields.

Consider asserting raw_message_url and events explicitly (even as nil) so the claim and coverage match.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/email_log_message_spec.rb` around lines 27 - 29, The spec "it
'creates a message with all attributes'" currently uses expect(message).to
have_attributes(attributes) but the attributes hash omits optional fields;
update the test so the claim matches reality by adding raw_message_url and
events to the asserted attributes (set them to nil if that's the expected value)
or add explicit expectations like expect(message.raw_message_url).to be_nil and
expect(message.events).to be_nil; refer to the test subject `message`, the
example name, and the `attributes` hash when making the change.

examples/email_logs_api.rb (1)

3-11: Consider reordering to favor environment variables as the primary approach.

The example hardcodes credentials first, then shows the environment variable approach as a comment. For better security practices in example code, consider swapping these:

♻️ Suggested reordering

-account_id = 3229
-client = Mailtrap::Client.new(api_key: 'your-api-key')
-email_logs = Mailtrap::EmailLogsAPI.new(account_id, client)
-
 # Set your API credentials as environment variables
 # export MAILTRAP_API_KEY='your-api-key'
 # export MAILTRAP_ACCOUNT_ID=your-account-id
-#
-# email_logs = Mailtrap::EmailLogsAPI.new
+email_logs = Mailtrap::EmailLogsAPI.new
+
+# Or manually pass credentials:
+# account_id = 3229
+# client = Mailtrap::Client.new(api_key: 'your-api-key')
+# email_logs = Mailtrap::EmailLogsAPI.new(account_id, client)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@examples/email_logs_api.rb` around lines 3 - 11, Swap the example to use
environment variables as the primary credential approach: instantiate
Mailtrap::Client and Mailtrap::EmailLogsAPI using ENV['MAILTRAP_API_KEY'] and
ENV['MAILTRAP_ACCOUNT_ID'] (or call Mailtrap::EmailLogsAPI.new with no args if
that constructor reads ENV) at the top, and move the hardcoded account_id and
Mailtrap::Client.new(api_key: 'your-api-key') example into a commented
“alternative” section; update the adjacent comments to show export instructions
and mention the hardcoded snippet as an optional fallback.

lib/mailtrap/email_logs_api.rb (1)

104-110: ⚠️ Potential issue | 🟠 Major

Prevent one unknown event type from aborting full response mapping.

At Lines 104-110, EmailLogEventDetails.build exceptions bubble up. If the API introduces a new event_type, #list/#get can fail entirely instead of returning the rest of the message data.

Proposed fix

 def build_events(events_array)
   Array(events_array).map do |e|
+    details = begin
+      EmailLogEventDetails.build(e[:event_type], e[:details] || {})
+    rescue ArgumentError
+      nil
+    end
+
     EmailLogEvent.new(
       event_type: e[:event_type],
       created_at: e[:created_at],
-      details: EmailLogEventDetails.build(e[:event_type], e[:details])
+      details: details
     )
   end
 end

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/email_logs_api.rb` around lines 104 - 110, build_events
currently maps events_array and calls EmailLogEventDetails.build for each event
which can raise and abort the whole mapping; modify build_events (the method
that constructs EmailLogEvent instances) to rescue exceptions raised by
EmailLogEventDetails.build for each individual event, log or record the error,
and fall back to a safe default (e.g. nil or an UnknownEmailLogEventDetails
placeholder) for the details so a single unknown/new event_type does not break
the entire response; keep the rest of the EmailLogEvent construction unchanged.

spec/mailtrap/email_log_message_spec.rb (1)

4-22: Consider adding optional attributes to test coverage.

The test attributes hash does not include raw_message_url and events fields, which are part of EmailLogMessage according to the API response mapping. While these fields are optional (returned only from the get endpoint), adding them to the test would ensure complete coverage of the DTO's initialization behavior.

💡 Suggested addition

       template_variables: {},
       opens_count: 2,
-      clicks_count: 1
+      clicks_count: 1,
+      raw_message_url: 'https://example.com/raw',
+      events: []
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/email_log_message_spec.rb` around lines 4 - 22, The spec's
attributes hash used to initialize EmailLogMessage is missing optional fields
raw_message_url and events; update the let(:attributes) hash in
spec/mailtrap/email_log_message_spec.rb to include raw_message_url (string URL)
and events (array of event hashes or empty array) so EmailLogMessage
initialization and its handling of optional API response fields are covered by
the test.

spec/mailtrap/email_log_event_details_spec.rb (1)

3-62: Consider adding test coverage for remaining event types.

The tests cover delivery, click, and bounce event types, but based on the implementation context, there are additional event types: open, spam, unsubscribe, reject, and suspension. While the current tests demonstrate the pattern works, adding at least one test for each struct type would provide more comprehensive coverage and document the expected attributes for each.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/email_log_event_details_spec.rb` around lines 3 - 62, Add tests
for the remaining event types by extending the Mailtrap::EmailLogEventDetails
spec: for each event_type 'open', 'spam', 'unsubscribe', 'reject', and
'suspension' call described_class.build(event_type, payload) and assert the
returned object is the appropriate struct (described_class::Open, ::Spam,
::Unsubscribe, ::Reject, ::Suspension) and has the expected attributes for that
type (mirror the pattern used for Delivery/Click/Bounce tests), using be_a(...)
and have_attributes(...) to verify attribute names and example values; keep the
tests consistent with existing examples so they validate .build behavior for all
known DETAIL_STRUCTS.

lib/mailtrap/email_logs_api.rb (1)

13-13: Unused response_class assignment.

The response_class is set to EmailLogMessage but is never used since this class implements custom list and get methods instead of using base_list and base_get from BaseAPI. Consider removing this line to avoid confusion, or add a comment explaining if it's intended for future use or external introspection.

🧹 Proposed removal

   class EmailLogsAPI
     include BaseAPI

-    self.response_class = EmailLogMessage
-
     # Lists email logs with optional filters and cursor-based pagination.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/email_logs_api.rb` at line 13, The file sets response_class =
EmailLogMessage but that assignment is unused because the class implements
custom list and get methods instead of leveraging BaseAPI's base_list/base_get;
remove the unused response_class = EmailLogMessage line from the EmailLogsAPI
class (or alternatively add a concise comment above it indicating it's
intentional/for external introspection) so the declaration no longer confuses
readers — search for response_class, EmailLogMessage, list, get, BaseAPI,
base_list and base_get in the EmailLogsAPI implementation to locate and apply
the change.

lib/mailtrap/email_log_event_details.rb (1)

53-58: ⚠️ Potential issue | 🟠 Major

Guard hash.slice against nil details to avoid runtime crashes.

At Line 57, hash.slice(...) will raise when details is nil. Coerce to {} before slicing.

Proposed fix

 def self.build(event_type, hash)
   struct_class = DETAIL_STRUCTS[event_type.to_s]
   raise ArgumentError, "Unknown event_type: #{event_type.inspect}" unless struct_class

-  attrs = hash.slice(*struct_class.members)
+  details_hash = hash.is_a?(Hash) ? hash : {}
+  attrs = details_hash.slice(*struct_class.members)
   struct_class.new(**attrs)
 end

#!/bin/bash
# Verify whether API fixtures contain null details and confirm current call-path behavior.

set -euo pipefail

echo "1) Look for null event details in VCR fixtures:"
rg -n '"details"\s*:\s*null' spec/fixtures/vcr_cassettes || true

echo
echo "2) Confirm EmailLogsAPI passes event details directly into builder:"
rg -n -C2 'EmailLogEventDetails\.build\(e\[:event_type\], e\[:details\]\)' lib/mailtrap/email_logs_api.rb

echo
echo "3) Confirm builder currently slices hash without nil coercion:"
rg -n -C2 'def self\.build|hash\.slice' lib/mailtrap/email_log_event_details.rb

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/email_log_event_details.rb` around lines 53 - 58, The build
method in EmailLogEventDetails uses hash.slice(...) which will raise if the
passed-in hash is nil; update the code in self.build to coerce the incoming
hash/details to an empty hash (e.g., using hash = (hash || {}) or hash.to_h)
before calling slice so attrs = hash.slice(*struct_class.members) is safe;
ensure you still raise the ArgumentError when DETAIL_STRUCTS[event_type.to_s] is
missing and construct the struct_class.new(**attrs) as before.

spec/mailtrap/email_logs_api_spec.rb (1)

6-7: ⚠️ Potential issue | 🟡 Minor

Make this VCR spec deterministic by removing ENV-driven IDs in expectations.

Line 6 can drift from the hardcoded path on Lines 55-56, which can cause cassette mismatches/flaky specs. Use a fixed account_id in this file and interpolate it in the expectation path.

Proposed fix

-  let(:account_id) { ENV.fetch('MAILTRAP_ACCOUNT_ID', 1_111_111) }
-  let(:client) { Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY', 'local-api-key')) }
+  let(:account_id) { 1_111_111 }
+  let(:client) { Mailtrap::Client.new(api_key: 'local-api-key') }

@@
-        expect(client).to have_received(:get).with(
-          '/api/accounts/1111111/email_logs',
+        expect(client).to have_received(:get).with(
+          "/api/accounts/#{account_id}/email_logs",
           {

Also applies to: 55-56

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/email_logs_api_spec.rb` around lines 6 - 7, The spec is
non-deterministic because account_id is pulled from ENV; replace the
ENV.fetch('MAILTRAP_ACCOUNT_ID', 1_111_111) usage by setting a fixed literal
account_id (e.g. 1111111) in the spec and update any expectations that build the
request path (the expectation that references the path using account_id around
the Mailtrap::Client call) to interpolate that same fixed account_id so the VCR
cassette path matches deterministically; ensure the let(:account_id) value and
the expectation path referencing account_id (used with client/EmailLogs API
requests) use the identical fixed value.

spec/mailtrap/email_log_message_spec.rb (1)

4-29: This spec doesn't cover the full DTO surface.

raw_message_url and events are part of Mailtrap::EmailLogMessage, but they aren't included in attributes, so the example name currently overstates what it verifies. Add them here or rename the example to reflect a partial attribute check.

♻️ Suggested update

       template_variables: {},
       opens_count: 2,
-      clicks_count: 1
+      clicks_count: 1,
+      raw_message_url: 'https://example.com/raw/test-message.eml',
+      events: [
+        Mailtrap::EmailLogEvent.new(
+          event_type: 'delivery',
+          created_at: '2025-01-15T10:30:01Z'
+        )
+      ]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/email_log_message_spec.rb` around lines 4 - 29, The spec's
attributes hash used to initialize Mailtrap::EmailLogMessage is missing
raw_message_url and events, so change the attributes used by described_class.new
(the let(:attributes) hash) to include keys for raw_message_url and events with
representative values, ensuring the subject(:message) test can truly assert all
DTO fields via expect(message).to have_attributes(attributes); alternatively, if
you prefer not to add them, update the example description (the it string) to
indicate it checks only a subset of attributes rather than "all attributes".